VPR-64 feat(phone): schoolwide and unit phone lists by bniedzie · Pull Request #323 · ucdavis/VIPER

bniedzie · 2026-08-25T19:05:54Z

This PR migrates the schoolwide and Dean's Office phone lists from Viper 1. Viewing the lists requires only basic permissions, while specific permissions allow users to edit and maintain the lists. The lists are now housed in the new Personnel area.

The migration makes the following functional changes from the Viper 1 version:

Rather than using the PhoneList database, creates a phones schema in the VIPER database.
The database structure is different, normalizing the existing data. Phone is now stored at a person level, meaning that the same person will have the same data every time they occur across lists. Name data now comes from the user.Person table, which more easily handles name changes. The primary person identifier is now IAM ID rather than Mothra ID, per an initial meeting with Brandon.
Change history is now tracked, rather than showing only the latest changes. This fixes a bug where deletions would not affect the last modified date shown to users.
A new SVMSecure.PhoneLists.SVMMaintain permission guards editing the SVM list. In Viper 1, anyone with the link could make changes.
Removed supervisor from the VMDO data, as it is not surfaced anywhere. This can be pulled from existing UCPath data if needed at a later date.
Generalized the VMDO table to support the creation of arbitrary department/unit phone lists. These are keyed by a code (e.g., VMDO), allowing the list name to change without breaking links. Each allows separate permissions. VMDO is the only list currently present.
Changed the general Phone List view to display direct numbers to those with list maintenance permissions as well. Previously, these users could view the data on the maintenance page only.
Fixed a broken link in the general Phone List view.
Changed phone lists to be filterable and (for relevant fields) sortable.

This PR also does some refactoring around Person selection and dialog boxes. There should be no end user impact to CMS, but a few files are affected.

This PR requires schema changes to the Production database:

CREATE SCHEMA phones;


CREATE TABLE [phones].[Person](
	[PersonIam] [VARCHAR](10) NOT NULL,
	[Phone] [NVARCHAR](25) NOT NULL,
	[ModifiedDate] [DATETIME] NULL,
	[ModifiedBy] [VARCHAR](10) NULL,
	[DirectPhone] [NVARCHAR](25) NULL,
	[Office] [NVARCHAR](100) NULL,
 CONSTRAINT [PK_Person] PRIMARY KEY CLUSTERED 
(
	[PersonIam] ASC
)WITH (PAD_INDEX = OFF, STATISTICS_NORECOMPUTE = OFF, IGNORE_DUP_KEY = OFF, ALLOW_ROW_LOCKS = ON, ALLOW_PAGE_LOCKS = ON) ON [PRIMARY]
) ON [PRIMARY];


CREATE TABLE [phones].[PhoneList](
	[PhoneListId] [INT] IDENTITY(1,1) NOT NULL,
	[Name] [NVARCHAR](100) NOT NULL,
	[MaintainRole] [VARCHAR](100) NOT NULL,
	[Code] [NVARCHAR](20) NOT NULL,
 CONSTRAINT [PK_PhoneList] PRIMARY KEY CLUSTERED 
(
	[PhoneListId] ASC
)WITH (PAD_INDEX = OFF, STATISTICS_NORECOMPUTE = OFF, IGNORE_DUP_KEY = OFF, ALLOW_ROW_LOCKS = ON, ALLOW_PAGE_LOCKS = ON) ON [PRIMARY]
) ON [PRIMARY];

CREATE UNIQUE NONCLUSTERED INDEX [UX_PhoneList_Code] ON [phones].[PhoneList]
(
	[Code] ASC
)WITH (PAD_INDEX = OFF, STATISTICS_NORECOMPUTE = OFF, SORT_IN_TEMPDB = OFF, IGNORE_DUP_KEY = OFF, DROP_EXISTING = OFF, ONLINE = OFF, ALLOW_ROW_LOCKS = ON, ALLOW_PAGE_LOCKS = ON) ON [PRIMARY];


CREATE TABLE [phones].[PhoneListUnit](
	[PhoneListUnitId] [INT] IDENTITY(1,1) NOT NULL,
	[PhoneListId] [INT] NOT NULL,
	[Name] [NVARCHAR](100) NOT NULL,
	[SortOrder] [INT] NULL,
 CONSTRAINT [PK_PhoneListUnit] PRIMARY KEY CLUSTERED 
(
	[PhoneListUnitId] ASC
)WITH (PAD_INDEX = OFF, STATISTICS_NORECOMPUTE = OFF, IGNORE_DUP_KEY = OFF, ALLOW_ROW_LOCKS = ON, ALLOW_PAGE_LOCKS = ON) ON [PRIMARY]
) ON [PRIMARY];

ALTER TABLE [phones].[PhoneListUnit]  WITH CHECK ADD  CONSTRAINT [FK_PhoneListUnit_PhoneListId] FOREIGN KEY([PhoneListId])
REFERENCES [phones].[PhoneList] ([PhoneListId]);
ALTER TABLE [phones].[PhoneListUnit] CHECK CONSTRAINT [FK_PhoneListUnit_PhoneListId];


CREATE TABLE [phones].[PhoneListUnitPerson](
	[PhoneListUnitPersonId] [INT] IDENTITY(1,1) NOT NULL,
	[PhoneListUnitId] [INT] NOT NULL,
	[PersonIam] [VARCHAR](10) NOT NULL,
	[ListFirst] [BIT] NOT NULL,
	[IsActive] [BIT] NOT NULL,
	[ModifiedBy] [VARCHAR](10) NULL,
	[ModifiedDate] [DATETIME] NULL,
 CONSTRAINT [PK_PhoneListUnitPerson] PRIMARY KEY CLUSTERED 
(
	[PhoneListUnitPersonId] ASC
)WITH (PAD_INDEX = OFF, STATISTICS_NORECOMPUTE = OFF, IGNORE_DUP_KEY = OFF, ALLOW_ROW_LOCKS = ON, ALLOW_PAGE_LOCKS = ON) ON [PRIMARY]
) ON [PRIMARY];

ALTER TABLE [phones].[PhoneListUnitPerson] ADD  DEFAULT ((0)) FOR [ListFirst];
ALTER TABLE [phones].[PhoneListUnitPerson] ADD  DEFAULT ((1)) FOR [IsActive];
ALTER TABLE [phones].[PhoneListUnitPerson]  WITH CHECK ADD  CONSTRAINT [FK_PhoneListUnitPerson_PersonIam] FOREIGN KEY([PersonIam])
REFERENCES [phones].[Person] ([PersonIam]);
ALTER TABLE [phones].[PhoneListUnitPerson] CHECK CONSTRAINT [FK_PhoneListUnitPerson_PersonIam];
ALTER TABLE [phones].[PhoneListUnitPerson]  WITH CHECK ADD  CONSTRAINT [FK_PhoneListUnitPerson_PhoneListUnitId] FOREIGN KEY([PhoneListUnitId])
REFERENCES [phones].[PhoneListUnit] ([PhoneListUnitId]);
ALTER TABLE [phones].[PhoneListUnitPerson] CHECK CONSTRAINT [FK_PhoneListUnitPerson_PhoneListUnitId];


CREATE TABLE [phones].[SVMSection](
	[SectionId] [INT] IDENTITY(1,1) NOT NULL,
	[Name] [NVARCHAR](100) NULL,
	[IncludeAbbrv] [BIT] NOT NULL,
	[DirectorTitle] [NVARCHAR](50) NOT NULL,
	[SortOrder] [INT] NULL,
	[UnitName] [NVARCHAR](50) NULL,
 CONSTRAINT [PK_SVMSection] PRIMARY KEY CLUSTERED 
(
	[SectionId] ASC
)WITH (PAD_INDEX = OFF, STATISTICS_NORECOMPUTE = OFF, IGNORE_DUP_KEY = OFF, ALLOW_ROW_LOCKS = ON, ALLOW_PAGE_LOCKS = ON) ON [PRIMARY]
) ON [PRIMARY];


CREATE TABLE [phones].[SVMUnit](
	[UnitId] [INT] IDENTITY(1,1) NOT NULL,
	[SectionId] [INT] NOT NULL,
	[Name] [NVARCHAR](100) NOT NULL,
	[Abbrv] [NVARCHAR](20) NULL,
	[SortOrder] [INT] NULL,
	[Fax] [NVARCHAR](25) NULL,
	[ModifiedBy] [VARCHAR](10) NULL,
	[ModifiedDate] [DATETIME] NULL,
 CONSTRAINT [PK_SVMUnit] PRIMARY KEY CLUSTERED 
(
	[UnitId] ASC
)WITH (PAD_INDEX = OFF, STATISTICS_NORECOMPUTE = OFF, IGNORE_DUP_KEY = OFF, ALLOW_ROW_LOCKS = ON, ALLOW_PAGE_LOCKS = ON) ON [PRIMARY]
) ON [PRIMARY];

ALTER TABLE [phones].[SVMUnit]  WITH CHECK ADD  CONSTRAINT [FK_SVMUnit_SectionId] FOREIGN KEY([SectionId])
REFERENCES [phones].[SVMSection] ([SectionId]);
ALTER TABLE [phones].[SVMUnit] CHECK CONSTRAINT [FK_SVMUnit_SectionId];


CREATE TABLE [phones].[SVMUnitPerson](
	[UnitPersonId] [INT] IDENTITY(1,1) NOT NULL,
	[UnitId] [INT] NOT NULL,
	[PersonIam] [VARCHAR](10) NOT NULL,
	[ModifiedDate] [DATETIME] NULL,
	[ModifiedBy] [VARCHAR](10) NULL,
	[Office] [NVARCHAR](50) NULL,
	[PosType] [NVARCHAR](25) NULL,
	[Interim] [NVARCHAR](10) NULL,
	[IsActive] [BIT] NOT NULL,
 CONSTRAINT [PK_SVMUnitPerson] PRIMARY KEY CLUSTERED 
(
	[UnitPersonId] ASC
)WITH (PAD_INDEX = OFF, STATISTICS_NORECOMPUTE = OFF, IGNORE_DUP_KEY = OFF, ALLOW_ROW_LOCKS = ON, ALLOW_PAGE_LOCKS = ON) ON [PRIMARY]
) ON [PRIMARY];

ALTER TABLE [phones].[SVMUnitPerson] ADD  DEFAULT ((1)) FOR [IsActive];
ALTER TABLE [phones].[SVMUnitPerson]  WITH CHECK ADD  CONSTRAINT [FK_SVMUnitPerson_PersonId] FOREIGN KEY([PersonIam])
REFERENCES [phones].[Person] ([PersonIam]);
ALTER TABLE [phones].[SVMUnitPerson] CHECK CONSTRAINT [FK_SVMUnitPerson_PersonId];
ALTER TABLE [phones].[SVMUnitPerson]  WITH CHECK ADD  CONSTRAINT [FK_SVMUnitPerson_UnitId] FOREIGN KEY([UnitId])
REFERENCES [phones].[SVMUnit] ([UnitId]);
ALTER TABLE [phones].[SVMUnitPerson] CHECK CONSTRAINT [FK_SVMUnitPerson_UnitId];


CREATE TABLE [phones].[SVMFrequentNumber](
	[NumberID] [INT] IDENTITY(1,1) NOT NULL,
	[Label] [NVARCHAR](100) NOT NULL,
	[Phone] [NVARCHAR](25) NOT NULL,
	[SortOrder] [INT] NULL,
	[ModifiedBy] [VARCHAR](10) NULL,
	[ModifiedDate] [DATETIME] NULL,
	[IsActive] [BIT] NOT NULL,
 CONSTRAINT [PK_SVMFrequentNumber] PRIMARY KEY CLUSTERED 
(
	[NumberID] ASC
)WITH (PAD_INDEX = OFF, STATISTICS_NORECOMPUTE = OFF, IGNORE_DUP_KEY = OFF, ALLOW_ROW_LOCKS = ON, ALLOW_PAGE_LOCKS = ON) ON [PRIMARY]
) ON [PRIMARY];

ALTER TABLE [phones].[SVMFrequentNumber] ADD  DEFAULT ((1)) FOR [IsActive];

This PR requires creating a new permission on Production: SVMSecure.PhoneLists.SVMMaintain.

This PR requires running the migration script .\RunMigrateData.bat Production for a dry run, and then .\RunMigrateData.bat Production --apply to migrate data into the new schema.

codecov-commenter · 2026-08-25T19:07:14Z

Bundle Report

Changes will increase total bundle size by 999 bytes (0.04%) ⬆️. This is within the configured threshold ✅

Detailed changes

Bundle name	Size	Change
viper-frontend-esm	2.29MB	999 bytes (0.04%) ⬆️

Affected Assets, Files, and Routes:

view changes for bundle: viper-frontend-esm

Assets Changed:

Asset Name	Size Change	Total Size	Change (%)
`assets/Files-*.js`	946 bytes	23.52kB	4.19%
`assets/ViperFetch-*.js`	53 bytes	11.22kB	0.47%

Files in assets/Files-*.js:

./src/CMS/components/PersonSelector.vue → Total Size: 150 bytes
./src/CMS/components/FileFormDialog.vue → Total Size: 150 bytes

codecov-commenter · 2026-08-25T19:08:22Z

Codecov Report

❌ Patch coverage is 94.05346% with 109 lines in your changes missing coverage. Please review.
✅ Project coverage is 44.15%. Comparing base (c6f64b5) to head (d4aaacb).

Files with missing lines	Patch %	Lines
...ersonnel/Services/PhoneSVMFrequentNumberService.cs	84.00%	6 Missing and 6 partials ⚠️
VueApp/src/Personnel/pages/PhoneListMaintain.vue	84.12%	5 Missing and 5 partials ⚠️
...s/Personnel/Controllers/PhoneListUnitController.cs	84.37%	8 Missing and 2 partials ⚠️
...el/Controllers/PhoneSVMFrequentNumberController.cs	70.00%	9 Missing ⚠️
VueApp/src/Personnel/composables/svm-data-fetch.ts	91.39%	3 Missing and 5 partials ⚠️
VueApp/src/Personnel/components/PersonSelector.vue	53.33%	7 Missing ⚠️
...b/Areas/Personnel/Services/PhoneListUnitService.cs	96.33%	2 Missing and 5 partials ⚠️
.../Personnel/components/PhoneListAddRecordDialog.vue	82.35%	5 Missing and 1 partial ⚠️
VueApp/src/Personnel/pages/SVMPhonesMaintain.vue	92.64%	2 Missing and 3 partials ⚠️
...src/Personnel/composables/use-add-record-dialog.ts	88.88%	3 Missing and 1 partial ⚠️
... and 15 more

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #323      +/-   ##
==========================================
+ Coverage   42.38%   44.15%   +1.76%     
==========================================
  Files         994     1056      +62     
  Lines       49877    51651    +1774     
  Branches     5887     6070     +183     
==========================================
+ Hits        21142    22804    +1662     
- Misses      27798    27875      +77     
- Partials      937      972      +35

Flag	Coverage Δ
backend	`41.70% <95.43%> (+1.33%)`	⬆️
frontend	`62.44% <91.88%> (+3.48%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
VueApp/src/CMS/components/PersonSelector.vue	`28.57% <100.00%> (-38.10%)`	⬇️
...ueApp/src/Personnel/components/ModifiedSummary.vue	`100.00% <100.00%> (ø)`
...pp/src/Personnel/components/PhoneListUnitTable.vue	`100.00% <100.00%> (ø)`
...pp/src/Personnel/components/RecordActionButton.vue	`100.00% <100.00%> (ø)`
...pp/src/Personnel/components/SVMAddRecordDialog.vue	`100.00% <100.00%> (ø)`
...rc/Personnel/components/SVMFrequentNumberTable.vue	`100.00% <100.00%> (ø)`
.../src/Personnel/components/SVMPhoneSectionTable.vue	`100.00% <100.00%> (ø)`
...src/Personnel/composables/phone-list-data-fetch.ts	`100.00% <100.00%> (ø)`
...App/src/Personnel/composables/use-person-helper.ts	`100.00% <100.00%> (ø)`
VueApp/src/Personnel/router/index.ts	`100.00% <100.00%> (ø)`
... and 55 more

... and 2 files with indirect coverage changes

rlorenzo · 2026-08-25T22:44:05Z

@coderabbitai full review

coderabbitai · 2026-08-25T22:44:11Z

⚠️ Action not completed

Review skipped: 122 files exceed the limit of 100.

Copilot

Pull request overview

This PR migrates the schoolwide (SVM) and Dean's Office (VMDO) phone lists from Viper 1 into a new Personnel area, backed by a new normalized phones schema in the VIPER database. Viewing requires basic SVMSecure permission, while a new SVMSecure.PhoneLists.SVMMaintain permission (and per-list MaintainRole) gates editing. It adds EF Core models/services/controllers plus a full Vue 3/Quasar SPA, and refactors shared person-search logic into a reusable PersonSearchHelper used by both CMS and Personnel.

Changes:

New phones schema + PhonesDbContext, EF models, area services and /api/phones/... controllers with dynamic per-list maintain permissions and direct-number masking.
New Personnel Vue SPA (lists, maintenance views, person selector, record dialogs) plus data-migration scripts from the legacy PhoneList database.
Shared PersonSearchHelper extracted and adopted by CMS's SearchPeople, forcing EF parameterization (ESCAPE clause) to prevent per-term query plans and %/_ wildcard injection.

Reviewed changes

Copilot reviewed 121 out of 122 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
web/Viper.csproj	Excludes `Areas\Personnel\Scripts\**` (separate migration project) from the web build, mirroring the Effort area.
web/Program.cs	Registers `PhonesDbContext`, adds `Personnel` SPA name and the Personnel services namespace to Scrutor registration.
web/Classes/Utilities/PersonSearchHelper.cs	New shared expression-tree helper for name-search autocomplete with parameterized `Contains` matching.
web/Areas/Personnel/Services/PhoneSVMSectionService.cs	Read-only query for SVM sections, ordered with null-safe sort.
web/Areas/Personnel/Services/PhoneSVMFrequentNumberService.cs	CRUD + soft-delete for SVM frequent numbers, with modified-date tracking.
web/Areas/Personnel/Services/PhonePersonLookupService.cs	Looks up phone people by IAM IDs (direct number masked unless maintainer) and current-employee search.
web/Areas/Personnel/Services/PhonePermissionsService.cs	Resolves edit permission from the list's `MaintainRole` column.
web/Areas/Personnel/Controllers/PhonePersonController.cs	Person-picker endpoint merging Viper and phone data; uses `foreach`/`Add` where `.Select()` is preferred.
web/Areas/Personnel/Controllers/PhoneSVMModifiedDateController.cs	Returns latest SVM modified date; contains a comment typo ("Identfies").
web/Areas/Personnel/Models/, VueApp/src/Personnel/*	New EF models/DTOs/Mapperly mapper and the Personnel Vue SPA (services, composables, components, tests).

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

+            List<string> iamIds = [];
+            foreach (ViperPerson result in viperResults)
+            {
+                iamIds.Add(result.IamId);
+            }


+        private readonly PhoneSVMUnitService _phoneSVMUnitService = phoneSVMUnitService;
+
+        /// <summary>
+        /// Identfies when frequent numbers were last modified.


rlorenzo

Solid work, and the parts that are easy to get wrong are right: ResolveListForMaintain, VerifyUnitInList, and GetUnitPersonInList each re-scope by list rather than trusting the id in the request, with a test proving one list's role grants nothing on another. I ran the branch against dev, so the inline notes are reproductions. Four things block deployment, none of them in the code:

The DDL won't run. CREATE SCHEMA Inventory; should be phones, so every CREATE TABLE [phones].[...] fails. Four ALTER TABLE [phones].[SVMUnitPerson] CHECK CONSTRAINT statements also name the wrong table, and three run before that table exists.
The DDL is missing the unique index on PhoneList.Code, and dev already has it. UX_PhoneList_Code was added to dev by hand, so Production won't get it and a duplicate code would resolve arbitrarily, including for the permission check.
A permission is missing from the steps. VMDO's MaintainRole is SVMSecure.PhoneLists.VMDOMaintain, but only SVMMaintain is listed, so nobody could maintain VMDO.
The pages aren't reachable from the nav. MainNav.cs:29 and MiniNav/Default.cshtml:105-109 still point Personnel at VIPER 1, though App.vue sets highlighted-top-nav="Personnel".

Also Home.vue needs a personnel-home CMS record per environment, or redirected non-maintainers land on a blank page. Everything else is inline, tagged minor where it's a nit rather than a fix I'd hold the PR for.

rlorenzo · 2026-08-26T07:07:21Z

+            dense
+            outlined
+            label="Location"
+            maxlength="100"


Writes to SVMUnitPerson.Office, which is NVARCHAR(50), so 51 to 100 chars 500s ("String or binary data would be truncated"). Needs maxlength="50" plus server-side length validation on the DTOs.

rlorenzo · 2026-08-26T07:07:21Z

+                await _phoneSVMUnitService.AddOrUpdateUnitData(unitId, request, ct);
+                return Ok(true);
+            }
+            catch (InvalidOperationException ex)


Only InvalidOperationException is caught, here and in every controller in the area, so DbUpdateException/SqlException become 500s. Catching DbUpdateException and mapping it to a 400 would cover the class.

rlorenzo · 2026-08-26T07:07:22Z

+        /// </summary>
+        public async Task AddUnitPersonData(int listId, PhoneListUnitDataRequest request, CancellationToken ct = default)
+        {
+            await VerifyUnitInList(listId, request.UnitId, ct);


request.EmployeeIam is never checked against users.Person, and no FK can enforce it since IamId isn't unique, so an unknown IAM returns 200 and writes a row that never renders. Needs an existence check before insert.

rlorenzo · 2026-08-26T07:07:22Z

+            // Avoid returning direct numbers except to users with permissions to access them.
+            // This data should only be returned for queries tied to a list for which the user
+            // has maintain permissions.
+            bool canAccessDirectNumber = list != null && _phonePermissionsService.CanMaintainList(list);


The gate checks you maintain listCode, but the results aren't scoped to that list, so a maintainer of list A gets DirectPhone for people only on list B. Scope the projection to list's members.

The current behavior is the desired/required one, but this case is a bit complicated.

Anyone with the ability to maintain any unit-specific list has the ability to add any user to it and so can gain access to the data anyways - scoping to the list doesn't actually achieve anything. Auto-populating direct phone data into the form for new additions to the list is dependent on this behavior, and users existing as part of multiple groups is viable (some admin staff already span multiple units) so auto-populating existing data to avoid overwriting existing values is important.

Ideally, we'd limit people returned by this query to those who have an appointment in the relevant unit, but Brandon mentioned that some units (including VMDO) do not fit nicely into the existing UCPath data, so this isn't viable with the current data quality.

What's most important is to not return the data to just anyone, or as part of the SVM queries where it is never needed.

I'll change the comment to be a bit more precise instead.

rlorenzo · 2026-08-26T07:07:22Z

+    }
+
+    function reportSaveError(res: { errors: string[] | null }) {
+        formError.value = res.errors?.[0] ?? `Failed to ${isEdit.value ? "save" : "upload"} ${recordLabel}`


errors[0] is EF's wrapper message and the useful one is last, so the truncation failure shows "See the inner exception for details" instead of the reason. Take the last entry, or join.

I can make this change, but all 25 other references to res.errors in the repo use res.errors?.[0] ?? for error reporting, and the errors reported here seem to be correct for both 4xx and 5xx errors. Is this a systematic issue across the repo?

rlorenzo · 2026-08-26T07:07:23Z

+        okColor: "negative",
+    })
+    if (!confirmed) return
+    let isError: boolean = false


Minor: isError plus let r does what the if/else in SVMPhonesMaintain.deleteRecord does directly. const either way.

rlorenzo · 2026-08-26T07:07:23Z

+        /// in that unit.
+        /// </summary>
+        [HttpGet("units")]
+        public async Task<ActionResult<List<PhoneListUnit>>> GetUnits(string code, CancellationToken ct = default)


Minor: returning the EF entity leaks the model into the API, so the TS types carry always-null nav props (phoneListUnit: null, unitPersons: null). PersonnelMapper is already here for AugmentedViperPerson.

rlorenzo · 2026-08-26T07:07:23Z

+
+namespace Viper.Areas.Personnel.Services
+{
+    public class PhonesPermissionsService(


Minor: file is PhonePermissionsService.cs, class is PhonesPermissionsService.

rlorenzo · 2026-08-26T07:07:23Z

+        const rows: PhoneListDisplayRecord[] = []
+        const cols: QTableProps["columns"] = [
+            { name: "name", label: "Name", field: "name", align: "left", sortable: true },
+            { name: "phone", label: "Phone", field: "phone", align: "left", sortable: false },


Minor: phone, direct phone, office, and fax are all sortable: false while the text columns sort, though the description says the lists are sortable now. Same in svm-data-fetch.ts:76.

This one is intentional, since sorting by these fields is not meaningful to end users. I'll update the description to clarify.

rlorenzo · 2026-08-26T07:07:23Z

+</template>
+
+<script setup lang="ts">
+import { searchPeopleOptions } from "../services/phone-person-options-service"


Minor: this component is ~85% identical to CMS/components/PersonSelector.vue. The search logic came out into use-person-search correctly; the component could follow with a couple of props.

VPR-64 feat(phone): schoolwide and unit phone lists

12baad2

VPR-64 feat(phone): improve test coverage and fix incorrect class name

3283e1b

rlorenzo requested a balanced review from Copilot August 25, 2026 22:50

Copilot started reviewing on behalf of rlorenzo August 25, 2026 22:50 View session

Copilot AI reviewed Aug 25, 2026

View reviewed changes

VPR-64 feat(phone): Fix code quality errors

d4aaacb

rlorenzo requested changes Aug 26, 2026

View reviewed changes

Uh oh!

Conversation

bniedzie commented Aug 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov-commenter commented Aug 25, 2026

Bundle Report

Affected Assets, Files, and Routes:

Assets Changed:

Uh oh!

codecov-commenter commented Aug 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

rlorenzo commented Aug 25, 2026

Uh oh!

coderabbitai Bot commented Aug 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

rlorenzo left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

bniedzie commented Aug 25, 2026 •

edited

Loading

codecov-commenter commented Aug 25, 2026 •

edited

Loading

coderabbitai Bot commented Aug 25, 2026 •

edited

Loading